[release/10.0] Fix InvalidCastException in OriginalValues.ToObject() with nested nullable complex properties#37703
Conversation
|
@copilot Update progress by rewriting the PR summary using this template: |
... Updated PR description with the requested template format. The issue number placeholder (#XXXXX) should be replaced with the actual issue number when available. |
test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs
Outdated
Show resolved
Hide resolved
test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs
Outdated
Show resolved
Hide resolved
test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs
Outdated
Show resolved
Hide resolved
test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs
Outdated
Show resolved
Hide resolved
…d nullable complex properties Fixes #37516 Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
9167798 to
60956cf
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes an InvalidCastException that occurs when calling OriginalValues.ToObject() or CurrentValues.ToObject() on entities with nested nullable complex properties where intermediate or leaf properties are null. This is a regression introduced in PR #37302 (v10.0.2).
Changes:
- Fixed
ArrayPropertyValues.ToObject()to correctly navigate nested complex property hierarchies when setting null values - Added
NavigateToDeclaringTypehelper to correctly read values from nested complex properties - Added comprehensive test coverage for various nested complex property scenarios
- Implemented quirk mode switch
Microsoft.EntityFrameworkCore.Issue37516for backward compatibility
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs | Adds SetNestedComplexPropertyValue to recursively navigate complex property chains when setting values, and NavigateToDeclaringType to navigate to declaring types when reading values. Includes quirk mode switch for backward compatibility. |
| test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs | Refactors existing tests to use StateManager directly instead of DbContext, and adds comprehensive test coverage for nested nullable complex properties in various configurations (single properties, collections, and nested combinations). |
Comments suppressed due to low confidence (1)
test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs:192
- The ClassData attribute should use
DataGenerator<bool?>instead ofDataGenerator<bool>to match the parameter typebool? useOriginalValuesand test all three cases (null, false, true) like the other test methods. Currently, this test only covers false and true, missing the Clone() code path (null case).
[ClassData(typeof(DataGenerator<bool>))]
| return root; | ||
| } | ||
|
|
||
| if (declaringType is not IComplexType ct) |
There was a problem hiding this comment.
nit: merge with the above condition
| } | ||
|
|
||
| var child = currentProperty.GetGetter().GetClrValue(instance); | ||
| if (child == null) |
There was a problem hiding this comment.
Especially because this is an arbitrary object where the equality operator might be overridden.
| if (child == null) | |
| if (child is null) |
Fixes #37516
Port of #37762
Description
When calling
context.Entry(...).OriginalValues.ToObject(), anInvalidCastExceptionis thrown when entities have nested nullable complex properties and any of those nested properties are null.Customer impact
This prevents customers from using
OriginalValues.ToObject()with nested nullable complex properties.Workaround: Customers can set the AppContext switch
Microsoft.EntityFrameworkCore.Issue37516totrueto revert to the old behavior, but this disables the nullable complex property feature added in 10.0.2.How found
Customer reported on 10.0.2.
Regression
Yes, this is a regression from 10.0.1, introduced in #37302
Testing
Tests added.
Risk
Low. Quirk mode added.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.